Repository navigation
Conversation
When a build emits chunks into subdirectories of outdir (multiple entrypoints with a root, or a [dir] naming template), the sourcemap sources array was computed relative to the outdir instead of the directory containing the .map file. Per the sourcemap spec (and esbuild), sources entries are resolved relative to the map file itself, so debuggers following them ended up at nonexistent out/<subdir>/<outdir-relative> paths. Compute chunk_abs_dir as output_dir + dirname(template) at post-process time. The content hash is not yet known there, but the hash formatter never emits a path separator, so a dummy value gives the correct directory depth. Fixes #3332
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
Reproduced on canary, fixed, PR #36601. |
|
Updated 10:26 PM PT - Jul 31st, 2026
✅ @robobun, your commit 7fdb4fa938e13ec5163a69017ad50940b30c844b passed in 🧪 To try this PR locally: bunx bun-pr 36601That installs a local version of the PR into your bun-36601 --bun |
join_abs_string_buf requires an absolute cwd; Bun.build stores a relative outdir verbatim. Anchor the join at FileSystem top_level_dir so a relative outdir is resolved the same way relative_platform_buf already does for the flat case. Adds a subprocess test that builds with outdir: 'dist' and asserts sources resolve from the map dir. Also guard the flat/inline tests against vacuously passing on an empty sources array.
There was a problem hiding this comment.
LGTM — both prior findings addressed.
What was reviewed:
- Verified 7fdb4fa's relative-
outdirfix:join_abs_string_bufresets its base when a part is absolute (resolve_path.rs:1712), so anchoring attop_level_diris correct for both absolute and relativeoutput_dir; new subprocess test covers it. - Verified the
rel_dirdummy-hash invariant:truncated_hash32alphabet is0-9a-zonly, no path separators. - Tests 2/3 now guard
sources.length >= 2before iterating. - The CSS-chunk call site remains untested (prior nit), but it's a mechanical mirror of the tested JS site and the shared helper is exercised.
Extended reasoning...
Overview
This PR fixes #3332: sourcemap sources entries were computed relative to outdir rather than the .map file's own directory, breaking debugger source lookup when chunks land in subdirectories. The fix introduces LinkerContext::source_map_chunk_abs_dir (joins output_dir with the chunk template's directory portion) and PathTemplate::rel_dir (expands the template with a dummy hash and takes dirname), then routes both post_process_js_chunk and post_process_css_chunk through it. Five files touched: two call sites, one new helper each in LinkerContext.rs and options.rs, and a new regression test file with four cases.
Prior review follow-up
I previously flagged (1) a 🔴 regression where join_abs_string_buf was called with a possibly-relative output_dir as its cwd, corrupting the base on POSIX and tripping a debug_assert on Windows, and (2) a 🟡 vacuous-loop pattern in tests 2/3. Both are addressed in the current diff: 7fdb4fa anchors the join at FileSystem::get().top_level_dir (always absolute) with [output_dir, rel_dir] as parts — I traced _join_abs_string_buf and confirmed an absolute part resets cwd (resolve_path.rs:1707-1722), so absolute-outdir behavior is unchanged. A subprocess test with outdir: "dist" covers the relative case. Tests 2/3 now assert sources.length >= 2 before the loop. The prior 🟡 CSS-test-coverage suggestion was not acted on; I consider it non-blocking since the CSS call site is a four-line copy of the tested JS call site and any bug in the shared helper would be caught by the JS tests.
Security risks
None. This affects only the relative-path strings written into emitted .js.map files. No untrusted input parsing, no filesystem writes beyond what Bun.build already does, no auth/crypto surface.
Level of scrutiny
Medium. Bundler sourcemap output is user-visible and production-relevant, but the change is narrow: the flat-outdir / no-outdir early-return preserves prior behavior byte-for-byte, and the new path only changes the base directory fed to the existing relative_alloc. I additionally verified the rel_dir dummy-hash trick is sound — truncated_hash32_bytes (fmt.rs:3466) uses a base32 alphabet with no / or \\, so the directory depth is invariant under the eventual hash.
Other factors
Four tests cover nested-external, flat-external, nested-inline, and relative-outdir. The PR description states existing snapshotSourceMap tests are unaffected (all flat outdir → early-return path). path_buffer_pool::get() returns a Drop guard, so no buffer leak. The author was responsive to review feedback, and the bug hunting system found nothing new on the current revision.
|
Closing this PR because it has been inactive for more than 90 days. |
|
Continued in #47548. It keeps the rule of this PR (the outdir plus the directory of the naming template, with a stand-in hash) and moves it into the source map writer. It also keeps the entries of executables and of standalone HTML with an inline map, which this PR changed by mistake. |
Fixes #3332.
What
When
Bun.buildemits chunks into subdirectories ofoutdir(multiple entrypoints under aroot, or a[dir]naming template), thesourcesarray in each.js.mapwas computed relative tooutdirinstead of the directory containing the.mapfile. Per the source map spec,sourcesentries are resolved relative to the map file (orsourceRoot), so debuggers following them ended up at nonexistent paths likeout/repos/livex/web/xxx/....Repro
Before:
out/repos/livex/web/src/index.js.mapcontains"sources": ["../xxx/node_modules/tiny-dep/index.js", ...], which resolves (from the map's directory) toout/repos/livex/web/xxx/...and does not exist.After:
"sources": ["../../../../../xxx/node_modules/tiny-dep/index.js", ...], which resolves to/repro/xxx/node_modules/tiny-dep/index.js.Cause
post_process_{js,css}_chunkpassedresolver.opts.output_diraschunk_abs_dirtogenerate_source_map_for_chunk, so everysourcesentry wasrelative(outdir, source)regardless of where inoutdirthe chunk actually lands.Fix
Compute
chunk_abs_dir = output_dir + dirname(chunk.template)at post-process time, matching esbuild'sfs.Dir(fs.Join(AbsOutputDir, TemplateToString(chunk.finalTemplate))).chunk.final_rel_pathis not yet populated (its content hash depends on post-process output), but the hash formatter never emits a path separator, so substituting a dummy value inPathTemplate::rel_diryields the correct directory depth even when[hash]appears in a directory segment of the naming template.When
rel_diris empty or.(flat outdir), oroutput_diris empty (nooutdir), the previous behavior is preserved.Verification
New
test/regression/issue/03332.test.tsbuilds a monorepo-style layout and asserts everysources[i]resolves from the map file's own directory for bothsourcemap: "external"andsourcemap: "inline". Fails on main withsource "../xxx/node_modules/tiny-dep/index.js" -> .../out/repos/livex/web/xxx/node_modules/tiny-dep/index.jsnot existing; passes with this change. ExistingsnapshotSourceMaptests inbundler_edgecase.test.ts,bundler_npm.test.ts, andinternal-sourcemap-roundtrip.test.tsare unaffected (all use a flat outdir).[review] gate passed · iteration 0 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file